Fix/critical bugs and improvements #6
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added initialization flags and promise tracking to prevent multiple storage listeners from being registered when init() is called multiple times. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Added await and per-tab error handling to prevent unhandled promise rejections when content scripts are not ready or tabs are closed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Wrapped Svelte component mount in try-catch with user-friendly error display to prevent silent failures. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replaced direct browser.storage.sync calls with loadFilters/saveFilters utilities for better separation of concerns and centralized error handling. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Moved debounce function from dom.ts to its own utility file and updated all consumers to import directly, removing unnecessary re-exports. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Set up Vitest testing framework with 21 tests covering filter name and value validation including GitHub qualifiers, edge cases, and error handling. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Removed duplicate storage.ts file (replaced by utils/storage.ts) and updated tsconfig to exclude test files from svelte-check. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This pull request adds comprehensive test infrastructure using Vitest and implements critical bug fixes and improvements to the browser extension's filter management system. The changes focus on improving error handling, preventing race conditions, and establishing a solid testing foundation.
Key Changes:
- Added Vitest testing framework with configuration and test utilities
- Refactored storage operations into dedicated utility functions with error handling
- Implemented race condition prevention in filter store initialization
- Enhanced error handling in messaging and content script mounting
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.setup.ts | Adds browser API mocks for testing environment |
| vitest.config.ts | Configures Vitest with happy-dom environment and path aliases |
| tsconfig.json | Excludes test files from TypeScript compilation |
| src/lib/utils/validation.test.ts | Comprehensive test suite for filter validation logic |
| src/lib/utils/storage.ts | Extracted storage operations with error handling from old storage.ts |
| src/lib/utils/debounce.ts | Moved debounce utility from dom.ts with improved documentation |
| src/lib/utils/dom.ts | Removed debounce function (moved to separate file) |
| src/lib/utils/messaging.ts | Added try-catch error handling for individual tab message failures |
| src/entrypoints/content.ts | Added try-catch wrapper with user-friendly error display |
| src/lib/stores/filters.ts | Improved initialization with race condition prevention and debounced listener |
| src/lib/storage.ts | Deleted old storage file (logic moved to stores and utils) |
| src/entrypoints/content/GithubPRFilter.svelte | Updated import to use new debounce location |
| src/entrypoints/background.ts | Refactored to use new storage utilities and fixed default filter type |
| package.json | Added Vitest, @vitest/ui, and happy-dom dependencies with test scripts |
| pnpm-lock.yaml | Updated lock file with new test dependencies |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
src/lib/utils/messaging.ts:34
- The newly added
broadcastFilterTogglefunction and its error handling logic lack test coverage. Since there's already a test file for validation.ts in the utils directory, messaging utilities should have corresponding tests to ensure they correctly handle broadcasting to multiple tabs, individual tab failures, and query errors.
export async function broadcastFilterToggle(filter: PRFilter): Promise<void> {
try {
const tabs = await browser.tabs.query({
url: GITHUB_PATTERNS.PR_PAGE_QUERY,
});
const message = {
action: MESSAGE_ACTIONS.TOGGLE_FILTER,
filter,
} satisfies ToggleFilterMessage;
for (const tab of tabs) {
if (tab.id) {
try {
await browser.tabs.sendMessage(tab.id, message);
} catch (err) {
// Silently ignore errors for individual tabs (e.g., content script not ready, tab closed)
logger.debug(`Failed to send message to tab ${tab.id}:`, err);
}
}
}
} catch (err) {
logger.error("Failed to broadcast filter toggle:", err);
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 17 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 17 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 20 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…nitialization 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…y race condition guards 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 20 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 21 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.